Add Mixedbread AI Rerank support#140477
Conversation
2683926 to
6133d64
Compare
|
Pinging @elastic/search-inference-team (Team:Search - Inference) |
There was a problem hiding this comment.
Pull request overview
This pull request adds support for Mixedbread AI's rerank API to Elasticsearch's inference plugin. The implementation follows the established pattern for inference service providers and includes comprehensive test coverage.
Changes:
- Implements Mixedbread rerank service with model, request/response handling, and action creators
- Adds service settings and task settings with configurable parameters (top_n, return_documents)
- Registers the new service in InferencePlugin and InferenceNamedWriteablesProvider
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| MixedbreadService.java | Main service implementation for Mixedbread rerank with configuration and inference methods |
| MixedbreadRerankModel.java | Model class defining rerank-specific configuration and URI building |
| MixedbreadRerankRequest.java | Request builder for Mixedbread rerank API calls |
| MixedbreadRerankResponseEntity.java | Response parser for Mixedbread rerank API responses |
| MixedbreadRerankTaskSettings.java | Task-level settings (top_n, return_documents) |
| MixedbreadRerankServiceSettings.java | Service-level settings (model_id, rate limits) |
| MixedbreadActionCreator.java | Creates executable actions for rerank operations |
| MixedbreadConstants.java | Shared constants for field names and API paths |
| MixedbreadAccount.java | Account credentials and URI management |
| InferencePlugin.java | Registers the Mixedbread service factory |
| InferenceNamedWriteablesProvider.java | Registers named writeables for serialization |
| Test files (8 files) | Comprehensive test coverage for all components |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assertThat(thrownException.getMessage(), containsString("field [top_n] is not of the expected type")); | ||
| } | ||
|
|
||
| public void UpdatedTaskSettings_WithEmptyMap_ReturnsSameSettings() { |
There was a problem hiding this comment.
The test name has inconsistent capitalization. It should start with 'test' (lowercase) to match Java naming conventions and be consistent with other test methods in the same file.
| public void UpdatedTaskSettings_WithEmptyMap_ReturnsSameSettings() { | |
| public void testUpdatedTaskSettings_WithEmptyMap_ReturnsSameSettings() { |
|
|
||
| import java.util.Map; | ||
|
|
||
| import static org.elasticsearch.xpack.inference.services.jinaai.rerank.JinaAIRerankTaskSettingsTests.getTaskSettingsMap; |
There was a problem hiding this comment.
The test is incorrectly using a JinaAI import for the helper method. This should use the Mixedbread equivalent method 'MixedbreadRerankTaskSettingsTests.getTaskSettingsMap' instead of importing from JinaAI rerank task settings tests.
| import static org.elasticsearch.xpack.inference.services.jinaai.rerank.JinaAIRerankTaskSettingsTests.getTaskSettingsMap; | |
| import static org.elasticsearch.xpack.inference.services.mixedbread.rerank.MixedbreadRerankTaskSettingsTests.getTaskSettingsMap; |
| XContentParser.Token token = parser.currentToken(); | ||
| ensureExpectedToken(XContentParser.Token.START_OBJECT, token, parser); | ||
|
|
||
| positionParserAtTokenAfterField(parser, "data", "FAILED_TO_FIND_FIELD_TEMPLATE"); // TODO error message |
There was a problem hiding this comment.
There's a TODO comment with an incomplete error message placeholder. The error message template should be properly defined, as this is used when parsing fails if the 'data' field is not found in the response.
| positionParserAtTokenAfterField(parser, "data", "FAILED_TO_FIND_FIELD_TEMPLATE"); // TODO error message | |
| positionParserAtTokenAfterField(parser, "data", "Failed to find [data] field in Mixedbread rerank response"); |
| public Boolean getDoesReturnDocuments() { | ||
| return returnDocuments; | ||
| } | ||
|
|
||
| public Integer getTopNDocumentsOnly() { | ||
| return topNDocumentsOnly; | ||
| } | ||
|
|
||
| public Boolean getReturnDocuments() { | ||
| return returnDocuments; | ||
| } |
There was a problem hiding this comment.
The class has two redundant getter methods for the same field. Both 'getDoesReturnDocuments()' and 'getReturnDocuments()' return the same 'returnDocuments' field. One of these methods should be removed to avoid confusion and maintain clean API design.
There was a problem hiding this comment.
I agree, let's remove one of these.
|
|
||
| @Override | ||
| public int rerankerWindowSize(String modelId) { | ||
| // Cohere rerank model truncates at 4096 tokens https://docs.cohere.com/reference/rerank |
There was a problem hiding this comment.
The comment incorrectly mentions "Cohere rerank model" when this is a Mixedbread service implementation. The comment should be updated to reference Mixedbread's actual model token limits or window size documentation.
| // Cohere rerank model truncates at 4096 tokens https://docs.cohere.com/reference/rerank | |
| // Mixedbread rerank models currently support context windows of up to 4096 tokens (see Mixedbread documentation) |
There was a problem hiding this comment.
This is pointing to cohere, I think we want the numbers posted here: https://www.mixedbread.com/docs/models/reranking/mxbai-rerank-large-v2
Looks like the older models have a window size of 512. We should make this configurable though. Let's add an optional field to the service settings that can control this value and default it to 8k.
There was a problem hiding this comment.
The newly added settings field will need to be used in
@Override
public int rerankerWindowSize(String modelId) {
so we probably need to pass a model as a parameter instead of modelId but then I need to make refactoring impacting other services, namely to make the change in TransportGetRerankerWindowSizeAction and services overriding rerankerWindowSize
Should we do it or it's better to revert some changes and make it configurable via model_id like that?
MixedbreadService
```
private static final Map<String, Integer> RERANKERS_INPUT_SIZE = Map.of(
"mixedbread-ai/mxbai-rerank-xsmall-v1",
512,
"mixedbread-ai/mxbai-rerank-base-v1",
512,
"mixedbread-ai/mxbai-rerank-large-v1",
512
// Windows size.
// The v1 models: 512
// The v2 models: at least 8k
// https://www.mixedbread.com/docs/models/reranking/mxbai-rerank-large-v1
);
@Override
public int rerankerWindowSize(String modelId) {
Integer inputSize = RERANKERS_INPUT_SIZE.get(modelId);
return inputSize != null ? inputSize : DEFAULT_RERANKER_INPUT_SIZE_WORDS;
}
@jonathan-buttner @DonalEvans
There was a problem hiding this comment.
The window size is not something a user can configure, it's an unchanging property of the model, so we don't need to store it in service settings. The current approach of having a map with the model IDs that don't use the 8k default is fine, but the rerankerWindowSize() method returns the size in words, not in tokens, so we'll need to translate from the 512/8000 values in tokens to smaller values in words by multiplying by 0.75 and rounding down a bit, which is the approach we use for other providers.
For consistency, the PR that originally introduced this feature can be used as a guide, with 512 tokens translating to a window size of 300 words, and 8000 tokens translating to 5500 words.
...e/src/main/java/org/elasticsearch/xpack/inference/services/mixedbread/MixedbreadService.java
Show resolved
Hide resolved
|
|
||
| public class MixedbreadConstants { | ||
| public static final String VERSION_1 = "v1"; | ||
| public static final String RERANK_PATH = "rerank"; |
There was a problem hiding this comment.
The constant RERANK_PATH is defined as "rerank" here but in MixedbreadRerankModel.java it's defined as "reranking". This inconsistency could lead to incorrect API paths being constructed. These should be unified to use the same value.
| public static final String RERANK_PATH = "rerank"; | |
| public static final String RERANK_PATH = "reranking"; |
There was a problem hiding this comment.
Mixedbread supports both. I left "rerank". Done
There was a problem hiding this comment.
While both may work, the documentation for Mixedbread reranking uses reranking as the endpoint, so that's what we should be using.
jonathan-buttner
left a comment
There was a problem hiding this comment.
Thanks for putting this together, I left some feedback.
| private static final String INVALID_REQUEST_TYPE_MESSAGE = "Invalid request type: expected Mixedbread %s request but got %s"; | ||
|
|
||
| private static final ResponseHandler RERANK_HANDLER = new MixedbreadRerankResponseHandler("mixedbread rerank", (request, response) -> { | ||
| if ((request instanceof MixedbreadRerankRequest) == false) { |
There was a problem hiding this comment.
I don't believe we typically check the the request type unless we need to use it. If the response format is invalid, the parsing logic will throw an error which should be good enough. Let's remove the if-block.
There was a problem hiding this comment.
Thanks. I deleted the if-block
| ), | ||
| QueryAndDocsInputs.class | ||
| ); | ||
| var errorMessage = buildErrorMessage(TaskType.RERANK, model.getInferenceEntityId()); |
There was a problem hiding this comment.
Let's use the helper method constructFailedToSendRequestMessage. Take a look at OpenAiActionCreator for example usage.
There was a problem hiding this comment.
Cleaned up and used the helper method instead. Thx
| public static void decorateWithAuthHeader(HttpPost request, MixedbreadAccount account) { | ||
| request.setHeader(HttpHeaders.CONTENT_TYPE, XContentType.JSON.mediaType()); | ||
| request.setHeader(createAuthBearerHeader(account.apiKey())); | ||
| request.setHeader(new BasicHeader(REQUEST_SOURCE_HEADER, ELASTIC_REQUEST_SOURCE)); |
There was a problem hiding this comment.
Can you point me to documentation as to why we need this header?
There was a problem hiding this comment.
I used other implementations as references mainly Cohere, Jina AI and the ones you suggested in the comments.
It happened I ended up with an unnecessary header. Since I didn't find the one to be required.
This class is now deleted due to the changes related to other comments and this header is not used in my implementation.
| * Write 360 120 1-minute | ||
| * Update 480 160 1-minute | ||
| * Delete 240 80 1-minute | ||
| * <a href="https://www.mixedbread.com/api-reference/rate-limits">Rate Limiting</a>. |
There was a problem hiding this comment.
These rate limits are for their storage operations. I'm not really sure what that is. If you go to the pricing page we can see that the free tier is limited to 100 requests per minute: https://www.mixedbread.com/pricing
Can you update the value to 100 and add the url I linked?
There was a problem hiding this comment.
I updated the url and added the link. Thank you
| public static MixedbreadRerankServiceSettings fromMap(Map<String, Object> map, ConfigurationParseContext context) { | ||
| ValidationException validationException = new ValidationException(); | ||
|
|
||
| String url = extractOptionalString(map, URL, ModelConfigurations.SERVICE_SETTINGS, validationException); |
There was a problem hiding this comment.
Does mixedbread allow users to spin up deployments? From poking around it seems like requests are only made to https://api.mixedbread.com. Can we remove this? For testing we'll need a way to pass a local URL. For examples of how to do that take a look at https://github.com/elastic/elasticsearch/blob/main/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/mistral/MistralModel.java
We basically just need a setter on the base model class.
There was a problem hiding this comment.
Yep, seems it can be removed. I did it. Also added the the suggested method
...e/src/main/java/org/elasticsearch/xpack/inference/services/mixedbread/MixedbreadService.java
Show resolved
Hide resolved
| } | ||
|
|
||
| @Override | ||
| protected void validateInputType(InputType inputType, Model model, ValidationException validationException) { |
There was a problem hiding this comment.
I think we can remove this.
There was a problem hiding this comment.
I removed the implementation from the method
|
|
||
| @Override | ||
| public TransportVersion getMinimalSupportedVersion() { | ||
| return TransportVersion.minimumCompatible(); |
There was a problem hiding this comment.
Switch this to use the new style.
|
|
||
| @Override | ||
| public Set<TaskType> supportedStreamingTasks() { | ||
| return COMPLETION_ONLY; |
There was a problem hiding this comment.
We don't support streaming tasks for Mixedbread yet so let's remove this.
|
|
||
| @Override | ||
| public int rerankerWindowSize(String modelId) { | ||
| // Cohere rerank model truncates at 4096 tokens https://docs.cohere.com/reference/rerank |
There was a problem hiding this comment.
This is pointing to cohere, I think we want the numbers posted here: https://www.mixedbread.com/docs/models/reranking/mxbai-rerank-large-v2
Looks like the older models have a window size of 512. We should make this configurable though. Let's add an optional field to the service settings that can control this value and default it to 8k.
|
Also please add a change log entry. |
| * Apart from v1 all other models have a context length of at least 8k. | ||
| * Apart from v1 all other models have a context length of up to 32k. | ||
| * <a href="https://github.com/elastic/elasticsearch/pull/132169#issue-3276542497">Here</a> | ||
| * 8k tokens were converted into 5500 words, that's why the default window size is set to 22000 |
There was a problem hiding this comment.
This comment is a little confusing. It would be better to simply say that 32,000 tokens was converted to 22,000 words by multiplying by 0.75 to get 24,000 and then rounding down a little.
There was a problem hiding this comment.
I used the javadoc comment which is used in the newest PR, but deleted one link since it returns 404 after the Mixedbread web-site update. It's a bit more descriptive and also contains the line about multiplying and rounding. If needed I yet replace it real quick, just tell me
| @Override | ||
| public void testParseRequestConfig_CreatesACompletionModel() {} |
There was a problem hiding this comment.
Rather than overriding this method, it would be better to add this line to the base class method:
Assume.assumeTrue(testConfiguration.commonConfig().supportedTaskTypes().contains(TaskType.COMPLETION));
| @Override | ||
| public void testParseRequestConfig_CreatesACompletionModel() {} | ||
|
|
||
| private static void assertModel(Model model, TaskType taskType, boolean modelIncludesSecrets, ConfigurationParseContext parseContext) { |
There was a problem hiding this comment.
Deleted. Thanks
# Conflicts: # server/src/main/resources/transport/upper_bounds/9.4.csv
jonathan-buttner
left a comment
There was a problem hiding this comment.
Did some testing and it looks good, one last change on the internal parsing logic.
Also could you update your PR description to use top_n instead of top_k.
| static { | ||
| PARSER.declareInt(constructorArg(), new ParseField("index")); | ||
| PARSER.declareFloat(constructorArg(), new ParseField("score")); | ||
| PARSER.declareField( |
There was a problem hiding this comment.
I mention more details here: #140477 (comment)
TLDR let's only support input as type string or null. So I think we want:
PARSER.declareStringOrNull(optionalConstructorArg(), new ParseField("input"));
I think we can remove the Document stuff below.
There was a problem hiding this comment.
Done. Thank you
This reverts commit 2124aa9.
…' into Add-Mixedbread-AI-Rerank-support
# Conflicts: # server/src/main/resources/transport/upper_bounds/9.4.csv
|
@elasticmachine run elasticsearch-ci/part-1 |
* Add Mixedbread AI Rerank support * Add Mixedbread AI Rerank support tests * Apply spotless * Add Mixedbread AI Rerank support * Add action creator tests * Make windows size configurable * Address comments and add service tests * [CI] Update transport version definitions * Switch to new approach for transport version * Use ConstructingObjectParser * Address comments * [CI] Auto commit changes from spotless * Fix the test * Checkstyle fix * Fix the test * Clean up * Clean up * [CI] Update transport version definitions * ci: retrigger * [CI] Update transport version definitions * Address comments and refactor * [CI] Update transport version definitions * Address comments * [CI] Update transport version definitions * Adjust to a new model creation approach * Address comments * Use API key from SecretSettings * [CI] Update transport version definitions * Change area from ML to Inference * [CI] Update transport version definitions * Address comments * Address comments * Add parameterized tests * Apply spotless * fix tests * update transport version * add transport version and upper bound * Address comments * Update transport version * Use 'declareStringOrNull' for the input field * [CI] Auto commit changes from spotless * Correct MixedbreadRerankResponseEntity * Revert "Correct MixedbreadRerankResponseEntity" This reverts commit 2124aa9. * Correct MixedbreadRerankResponseEntity * address the comment * [CI] Auto commit changes from spotless * fix the test * Add version --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
RERANK
request put {{base-url}}/_inference/rerank/mixedbread
{ "service": "mixedbread", "service_settings": { "api_key": "{{mb-api-key}}", "model_id": "mixedbread-ai/mxbai-rerank-xsmall-v1" }, "task_settings": { "return_documents": true, "top_k": 1 } }response
{ "inference_id": "mixedbread", "task_type": "rerank", "service": "mixedbread", "service_settings": { "model_id": "mixedbread-ai/mxbai-rerank-xsmall-v1", "rate_limit": { "requests_per_minute": 240 } }, "task_settings": { "return_documents": true } }request post {{base-url}}/_inference/rerank/mixedbread
{ "input": ["Luke", "like", "leia", "chewy","r2d2", "star", "wars"], "query": "star wars main character", "top_n": 2, "return_documents": true }response
{ "rerank": [ { "index": 0, "relevance_score": 0.083740234, "text": "Luke" }, { "index": 2, "relevance_score": 0.06994629, "text": "leia" } ] }direct request post https://api.mixedbread.com/v1/reranking
{ "model": "mixedbread-ai/mxbai-rerank-xsmall-v1", "query": "Who is the author of To Kill a Mockingbird?", "input": [ "To Kill a Mockingbird is a novel by Harper Lee", "The novel Moby-Dick was written by Herman Melville", "Harper Lee, an American novelist", "Jane Austen was an English novelist", "The Harry Potter series written by British author J.K. Rowling", "The Great Gatsby, a novel written by American author F. Scott Fitzgerald" ], "top_k": 3, "return_input": true }response
}